Skip to content

Allow format_args! with a macro as first argument #4064

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Allow format_args! with a macro as first argument #4064

wants to merge 2 commits into from

Conversation

powerboat9
Copy link
Collaborator

Fixes #3896. FormatArgsPieces isn't great, but I don't see a better alternative for now.

@powerboat9
Copy link
Collaborator Author

powerboat9 commented Aug 12, 2025 •
edited
Loading

Now depends on #4068 #4074

@powerboat9
Improve libformat_parser FFI
9292100 
This should remove a use-after-free as well as simplify the FFI
interface.

gcc/rust/ChangeLog:

	* ast/rust-fmt.cc (Pieces::collect): Handle changes to ffi
	interface.
	(Pieces::~Pieces): Remove function definition.
	(Pieces::Pieces): Likewise.
	(Pieces::operator=): Likewise.
	* ast/rust-fmt.h: Include "optional.h".
	(rust_ffi_alloc): New extern "C" function declaration.
	(rust_ffi_dealloc): Likewise.
	(class FFIVec): New class.
	(class FFIOpt): Likewise.
	(RustHamster::RustHamster): New constructor accepting const
	std::string reference.
	(struct FormatSpec): Use FFIOpt.
	(struct PieceSlice): Remove struct.
	(struct RustString): Likewise.
	(struct FormatArgsHandle): Likewise.
	(collect_pieces): Change function signature.
	(clone_pieces): Likewise.
	(destroy_pieces): Remove extern "C" function declaration.
	(Pieces::~Pieces): Remove function declaration.
	(Pieces::operator=): Likewise.
	(Pieces::get_pieces): Handle changes to class fields.
	(Pieces::Pieces): Remove copy and move constructor declarations,
	adjust signature of remaining constructor declaration.
	(Pieces::pieces_vector): Remove member variable.
	(Pieces::handle): Likewise.
	(Pieces::data): Add member variable.
	* expand/rust-macro-builtins-asm.cc (expand_inline_asm_strings):
	Use references to avoid copying.

libgrust/ChangeLog:

	* libformat_parser/src/lib.rs (struct FFIVec): New.
	(trait StringLeakExt): Remove.
	(struct FFIOpt): New.
	(trait IntoFFI): Adjust implementation for Option.
	(struct RustHamster): Add lifetime and adjust conversion to and
	from &str.
	(enum Piece): Adjust definition to handle changes to
	RustHamster.
	(struct Argument): Likewise.
	(struct FormatSpec): Use FFIOpt and RustHamster.
	(enum Position): Use RustHamster.
	(enum Count): Likewise.
	(struct PieceSlice): Replace with...
	(typedef PieceVec): ...this.
	(struct RustString): Remove.
	(struct FormatArgsHandle): Likewise.
	(fn collect_pieces): Adjust signature, greatly simplifying
	implementation.
	(fn clone_pieces): Likewise.
	(fn destroy_pieces): Remove.
	(trait LayoutExt): New.
	(fn rust_ffi_alloc): New.
	(fn rust_ffi_dealloc): New.

Signed-off-by: Owen Avery <[email protected]>
@powerboat9
Allow format_args! with a macro as first argument
86733e9 
Fixes Rust-GCC#3896. FormatArgsPieces
isn't great, but I don't see a better alternative for now.

gcc/rust/ChangeLog:

	* ast/rust-ast-visitor.cc (DefaultASTVisitor::visit): Visit
	first argument of FormatArgs.
	* ast/rust-ast.cc (FormatArgs::clone_expr_impl): Remove usage of
	std::cerr for debugging.
	* ast/rust-builtin-ast-nodes.h (enum class FormatArgsNewline):
	New enum.
	(class FormatArgsPieces): New class.
	(FormatArgs::Newline): Replace with using declaration
	referencing FormatArgsNewline.
	(FormatArgs::FormatArgs): Accept a FormatArgsPieces instance.
	(FormatArgs::get_template_arg): New member function.
	(FormatArgs::get_template): Handle changes to class fields.
	(FormatArgs::template_pieces): Rename to...
	(FormatArgs::template_arg): ...here and change type to
	FormatArgsPieces.
	* expand/rust-expand-format-args.cc: Include
	"rust-expand-visitor.h".
	(expand_format_first_arg): New static function.
	(expand_format_args): Change signature.
	* expand/rust-expand-format-args.h: Include "rust-system.h" and
	adjust include order.
	(class ExpandVisitor): Forward declare.
	(struct MacroExpander): Likewise.
	(expand_format_args): Change signature.
	* expand/rust-expand-visitor.cc: Include
	"rust-expand-format-args.h".
	(ExpandVisitor::visit): Attempt to expand FormatArgs.
	* expand/rust-expand-visitor.h: Include "rust-ast-full-decls.h".
	(ExpandVisitor::visit): New overload for FormatArgs.
	* expand/rust-macro-builtins-format-args.cc
	(FormatArgsInput::format_str): Rename to...
	(FormatArgsInput::format_expr): ...here and change type to
	std::unique_ptr<Expr>.
	(format_args_parse_arguments): Handle change to FormatArgsInput.
	(MacroBuiltin::format_args_handler): Likewise and handle changes
	to FormatArgs.

gcc/testsuite/ChangeLog:

	* rust/compile/format_args_concat.rs: New test.

Signed-off-by: Owen Avery <[email protected]>
CohenArthur
CohenArthur reviewed Aug 19, 2025
View reviewed changes
Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have you looked at reusing the code for expanding eager macro invocations for other builtin macros? I haven't reviewed your changes yet but it seems like a lot of trouble. I was hoping we could just reuse the existing infrastructure and create the AST::FormatArgs node only when there are no eager macro invocations left

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CohenArthur CohenArthur CohenArthur left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow eager macro invocations in format_args!()
2 participants
@powerboat9 @CohenArthur